Skip to content

fix(event_handler): raise more specific SerializationError exception for unsupported types in data validation #4415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

leandrodamascena
Copy link
Contributor

Issue number: #4300

Summary

Changes

The current implementation of the Data Validation does not natively support serializing some types of objects, including SQLAlchemy model objects and other custom classes that may not have a default serialization mechanism. This can lead to issues when trying to return these objects as the error is confuse.

In this PR, we are adding more robust exception handling and a clear message for cases where the object to be serialized is not supported.

Example of this implementation:

from typing import Any
from json import JSONEncoder
from dataclasses import is_dataclass, asdict
from sqlalchemy.orm.base import InstrumentedAttribute
from pydantic.main import ModelMetaclass

class UnsupportedObjectEncoder(JSONEncoder):
    def default(self, obj: Any) -> Any:
        if isinstance(obj, InstrumentedAttribute):
            return str(obj)
        elif is_dataclass(obj):
            return asdict(obj)
        elif isinstance(obj, ModelMetaclass):
            return obj.dict()
        else:
            try:
                return super().default(obj)
            except TypeError:
                # Handle unsupported objects by returning a default representation
                return str(obj)

def custom_serializer(obj: Any) -> str:
    return json.dumps(obj, separators=(",", ":"), cls=UnsupportedObjectEncoder)

app = APIGatewayRestResolver(enable_validation=True, serializer=custom_serializer)

User experience

Before this implementation, the customer received an incomprehensible error like:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/aws_lambda_powertools/event_handler/openapi/encoders.py", line 272, in _dump_other
backend-1  |
    data = dict(obj)
           ^^^^^^^^^
TypeError: cannot convert dictionary update sequence element #0 to a sequence
backend-1  |
During handling of the above exception, another exception occurred:
backend-1  |
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/aws_lambda_powertools/event_handler/openapi/encoders.py", line 276, in _dump_other
    data = vars(obj)

Now we've added a clear message in the exception

[ERROR] EncoderError: ('Unable to serializer the object <app.MyClass object at 0x7ffff27f45c0> as it is not a supported type. Error details: [TypeError("\'MyClass\' object is not iterable"), TypeError(\'vars() argument must have __dict__ attribute\')]', 'See: https://docs.powertools.aws.dev/lambda/python/latest/core/event_handler/api_gateway/#serializing-objects')

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Is this a breaking change?

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@leandrodamascena leandrodamascena requested a review from a team May 28, 2024 11:09
@boring-cyborg boring-cyborg bot added documentation Improvements or additions to documentation event_handlers tests labels May 28, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 28, 2024
@github-actions github-actions bot added bug Something isn't working and removed documentation Improvements or additions to documentation labels May 28, 2024
Copy link

codecov bot commented May 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.39%. Comparing base (e14e768) to head (b8c2bb6).
Report is 548 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4415      +/-   ##
===========================================
+ Coverage    96.38%   96.39%   +0.01%     
===========================================
  Files          214      219       +5     
  Lines        10030    10507     +477     
  Branches      1846     1944      +98     
===========================================
+ Hits          9667    10128     +461     
- Misses         259      267       +8     
- Partials       104      112       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@leandrodamascena leandrodamascena added documentation Improvements or additions to documentation openapi-schema and removed bug Something isn't working labels May 28, 2024
@github-actions github-actions bot added bug Something isn't working and removed documentation Improvements or additions to documentation labels May 28, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jun 4, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 4, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jun 4, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 4, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jun 4, 2024
@leandrodamascena leandrodamascena requested a review from sthulb June 4, 2024 12:09
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 4, 2024
Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggested changes in doc to make this more visible and easier to parse, and a note about encoders refactoring to singledispatch. In the ideal world, most of this logic would be shared this level of serialization would benefit other areas like idempotency to cut duplication.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jun 6, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 6, 2024
@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jun 6, 2024
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 6, 2024
@leandrodamascena
Copy link
Contributor Author

Ready to review again @sthulb and @heitorlessa

@heitorlessa
Copy link
Contributor

looking...

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last comment before merging - changing the docs section name.

@boring-cyborg boring-cyborg bot added the documentation Improvements or additions to documentation label Jun 10, 2024
@heitorlessa
Copy link
Contributor

heitorlessa commented Jun 10, 2024

LGTM. @sthulb I'm gonna resolve the merge conflict and get your approval to merge, as @leandrodamascena is on public holiday today

Update: nvm, he's working on it

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@heitorlessa heitorlessa changed the title fix(event_handler): exception handling for unsupported objects in data validation fix(event_handler): raise more specific SerializationError exception for unsupported types in data validation Jun 10, 2024
@heitorlessa heitorlessa merged commit f1437fd into aws-powertools:develop Jun 10, 2024
17 checks passed
@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working event_handlers openapi-schema size/L Denotes a PR that changes 100-499 lines, ignoring generated files. tests
Projects
None yet
3 participants